Make Telemetry API log record type generic#1098
Make Telemetry API log record type generic#1098raphael-theriault-swi wants to merge 2 commits intoaws:mainfrom
Conversation
Managed instances will not require semver breaking. We are a long ways off from release 2.x. With that in mind, do you have any thoughts about a semver-safe way to do it? I guess we just add a new API, deprecate the old, but allow continuing to use old until next release version? |
|
(Also, thanks for picking this up! This is definitely in an awkward spot right now) |
|
I'm thinking maybe an opt-in feature to enable the generic might be more elegant ? Then the feature can just be removed when a future |
|
The opt-in feature will work, but it seems a bit confusing to have a feature that shifts runtime behavior like this, especially for such a limited feature scope. Let me get a temperature check from the other maintainers on this and follow up. |
📬 Issue #, if available: #977
✍️ Description of changes:
This adds a generic type parameter to
LambdaTelemetryRecordfor logs to support structured JSON logs. I initially tried to implement theRawValuenewtype based solution suggested in #977, but it turns out you can't useRawValuewithin#[serde(flatten)].This is a breaking change given it introduces a new generic parameter, but IMO it's better than introducing a whole new set of duplicate types, especially given supporting managed instances may already require breakage soon.
Unlike suggested in #977, this doesn't lock users into either supporting text or JSON logs at compile time, since they can just use an untagged enum:
I also opted against adding a new struct for JSON logs since they don't have a fixed schema, some extension authors might want to use
serde_json::Value, some might want a more specialized type.🔏 By submitting this pull request
cargo +nightly fmt.cargo clippy --fix.